Adjusted code to add new endpoint #105
Conversation
WalkthroughThe changes introduce update functionality for company entities, including new API endpoints and command/handler classes. The company entity logic is refactored to centralize rating recalculation and add an update method. Supporting request and command types are renamed or adapted. Logging middleware and exception handling receive minor improvements, and some types are converted to records for value-based semantics. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant CompaniesController
participant Mediator
participant UpdateCompanyHandler
participant DatabaseContext
Client->>CompaniesController: PUT /companies/{companyId}
CompaniesController->>Mediator: Send UpdateCompanyCommand(companyId, body)
Mediator->>UpdateCompanyHandler: Handle(command)
UpdateCompanyHandler->>DatabaseContext: Find company by ID
UpdateCompanyHandler->>Company: Update(name, description, logoUrl, links)
UpdateCompanyHandler->>DatabaseContext: SaveChangesAsync()
UpdateCompanyHandler-->>Mediator: Unit
Mediator-->>CompaniesController: Unit
CompaniesController-->>Client: 200 OK
Poem
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Frontend Techinterview-space/frontend#76 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
src/Domain/Validation/Exceptions/EntityInvalidException.cs (1)
24-24: Redundant condition could be simplified.This condition is always true at this point in the code flow since line 18 already ensures errors.Count > 0.
- if (errors.Count != 0) + // No condition needed as errors.Count is always > 0 heresrc/Web.Api/Features/Companies/UpdateCompany/UpdateCompanyHandler.cs (1)
13-43: Well-implemented handler following CQRS pattern.This handler correctly implements the MediatR pattern for handling the company update command with proper separation of concerns:
- Database access is properly injected via constructor
- The handler performs appropriate validation and error handling
- Business logic is delegated to the domain entity through the
Updatemethod- Async/await is used correctly for database operations
One suggestion would be to add some logging for successful updates and potential errors.
Consider adding logging to track successful updates:
public async Task<Unit> Handle( UpdateCompanyCommand request, CancellationToken cancellationToken) { request.Body.ThrowIfInvalid(); var company = await _context.Companies.FirstOrDefaultAsync( c => c.Id == request.CompanyId, cancellationToken) ?? throw NotFoundException.CreateFromEntity<Company>(request.CompanyId); company.Update( request.Body.Name, request.Body.Description, request.Body.LogoUrl, request.Body.Links); await _context.TrySaveChangesAsync(cancellationToken); + _logger.LogInformation("Company {CompanyId} updated successfully", request.CompanyId); return Unit.Value; }src/Web.Api/Features/Companies/CompaniesController.cs (1)
57-57: Consider using PUT instead of POST for updates.The endpoint currently uses HTTP POST for the update operation. RESTful API conventions suggest using PUT for complete resource updates or PATCH for partial updates. Consider changing the HTTP method to better align with REST standards.
-[HttpPost("{companyId:guid}")] +[HttpPut("{companyId:guid}")] [HasAnyRole(Role.Admin)] public async Task<IActionResult> UpdateCompany(src/Domain/Entities/Companies/Company.cs (1)
167-173: Consider using a more generic error message.The error message in
NotDeletedOrFail()is specific to marking a review as outdated, but this method is called from multiple contexts (e.g.,AddReview,ApproveReview).Consider using a more generic message:
public void NotDeletedOrFail() { if (DeletedAt != null) { - throw new BadRequestException("Cannot mark a review as outdated."); + throw new BadRequestException("Cannot perform this operation on a deleted company."); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/Domain/Entities/Companies/Company.cs(4 hunks)src/Domain/Validation/Exceptions/EntityInvalidException.cs(1 hunks)src/Web.Api/Features/Companies/CompaniesController.cs(3 hunks)src/Web.Api/Features/Companies/CreateCompany/CreateCompanyCommand.cs(1 hunks)src/Web.Api/Features/Companies/CreateCompany/EditCompanyBodyRequest.cs(1 hunks)src/Web.Api/Features/Companies/UpdateCompany/UpdateCompanyCommand.cs(1 hunks)src/Web.Api/Features/Companies/UpdateCompany/UpdateCompanyHandler.cs(1 hunks)src/Web.Api/Features/Telegram/GetTelegramInlineUsageStats/TelegramInlineUsageChatDataItem.cs(1 hunks)src/Web.Api/Middlewares/LoggingMiddleware.cs(2 hunks)
🔇 Additional comments (15)
src/Web.Api/Features/Telegram/GetTelegramInlineUsageStats/TelegramInlineUsageChatDataItem.cs (1)
3-3: Good choice converting to a record type.Converting this class to a record provides value-based equality semantics and immutability, which is appropriate for this data transfer object. This enhances code quality with no behavioral changes.
src/Web.Api/Features/Companies/CreateCompany/EditCompanyBodyRequest.cs (1)
6-6: Good refactoring to support both create and update operations.Renaming from
CreateCompanyBodyRequesttoEditCompanyBodyRequestmakes this model more versatile and follows the DRY principle, allowing it to be used for both creating and updating companies.src/Domain/Validation/Exceptions/EntityInvalidException.cs (2)
14-15: LGTM - Improved method signature formatting.Breaking the parameter onto a new line improves readability for this method signature.
18-18: Good performance optimization using Count instead of Any().Using
Count == 0is more efficient than!Any()forICollection<T>as it avoids unnecessary LINQ operations.src/Web.Api/Features/Companies/CreateCompany/CreateCompanyCommand.cs (2)
8-8: Properly updated inheritance to use EditCompanyBodyRequest.The base class update aligns with the renaming of the request model while maintaining the same functionality.
11-11: Consistent parameter type update.Parameter type correctly updated to match the renamed request model.
src/Web.Api/Middlewares/LoggingMiddleware.cs (2)
22-23: Good addition of domain exceptions to the ignore list.Adding
EntityInvalidExceptionandNotFoundExceptionto the ignored exceptions list is appropriate as these represent expected business cases rather than actual errors that require detailed logging.
44-47: Improved logging with structured format.The updated logging now uses a structured format with a proper message template. This is a good practice as it:
- Makes logs more searchable and filterable
- Prevents log injection vulnerabilities
- Maintains exception details by passing the exception object as the first parameter
src/Web.Api/Features/Companies/UpdateCompany/UpdateCompanyHandler.cs (1)
29-32: Concise null checking with the null-coalescing operator.Good use of the null-coalescing operator combined with the throw expression to handle the case when the company isn't found. This is a clean and readable approach.
src/Web.Api/Features/Companies/UpdateCompany/UpdateCompanyCommand.cs (1)
7-20: Proper implementation of a command record.Using a record type for the command provides valuable immutability and value-based semantics. The command is well-structured with:
- Clear implementation of the IRequest interface
- Constructor-based initialization
- Read-only properties
This ensures the command data cannot be modified after creation, which helps prevent side effects during processing.
src/Web.Api/Features/Companies/CompaniesController.cs (2)
48-48: Consistent use of EditCompanyBodyRequest.The parameter type change from
CreateCompanyBodyRequesttoEditCompanyBodyRequestreflects the refactoring to use a single request type for both creation and updates, which promotes consistency and reusability.
58-70: Well-implemented update endpoint with proper authorization.The update endpoint correctly:
- Restricts access to admin users with the HasAnyRole attribute
- Receives the company ID from the route and update data from the body
- Uses the mediator pattern to dispatch the command
- Returns the result to the client
The implementation is consistent with other endpoints in the controller and follows the established patterns.
src/Domain/Entities/Companies/Company.cs (3)
111-116: Good refactoring to centralize rating calculation!The change to use the extracted
RecalculateRatingmethod is a good refactoring that follows the DRY principle and centralizes the rating calculation logic.
136-141: Refactoring improves consistency in rating history updates.The
MarkReviewAsOutdatedmethod now uses the centralizedRecalculateRatingmethod, making the code more maintainable and consistent with theApproveReviewmethod.
144-155: Well-designed centralized method for rating calculation.The new
RecalculateRatingmethod effectively centralizes the logic for computing ratings, which was previously duplicated. It properly updates bothRatingandReviewsCountproperties while returning the new rating value for immediate use.
| @@ -0,0 +1,20 @@ | |||
| using System; | |||
| using MediatR; | |||
| using Web.Api.Features.Companies.CreateCompany; | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Using the same request model for create and update operations.
The reuse of EditCompanyBodyRequest from the CreateCompany namespace for update operations promotes consistency and reduces duplication. This suggests the model has been renamed from its original CreateCompanyBodyRequest to better reflect its dual purpose.
🏁 Script executed:
#!/bin/bash
# Check if EditCompanyBodyRequest was renamed from CreateCompanyBodyRequest
git log -p -- "src/Web.Api/Features/Companies/CreateCompany/EditCompanyBodyRequest.cs" | grep -A 10 -B 10 "CreateCompanyBodyRequest"Length of output: 0
🏁 Script executed:
#!/bin/bash
# Locate the file that defines EditCompanyBodyRequest
file=$(rg -l "class EditCompanyBodyRequest" -n .)
echo "Found file: $file"
# Check its Git history (including renames) for occurrences of CreateCompanyBodyRequest
git log --follow --name-status -- "$file" | grep -E "^(R[0-9]+|[AMDR]\s)" | sed -n '1,20p'
git log --follow -p -- "$file" | grep -C 5 "CreateCompanyBodyRequest"Length of output: 87
🏁 Script executed:
#!/bin/bash
# 1. List all BodyRequest classes in the repository
echo "=== All BodyRequest class definitions ==="
rg --ignore-case -n "class .*BodyRequest" -t cs || true
echo
# 2. List files in the CreateCompany feature folder
echo "=== Files in src/Web.Api/Features/Companies/CreateCompany ==="
find src/Web.Api/Features/Companies/CreateCompany -maxdepth 1 -type f -name "*.cs" -printf '%f\n' || true
echo
# 3. Show the top of UpdateCompanyCommand.cs to inspect its usings
echo "=== UpdateCompanyCommand.cs imports ==="
update_file=$(rg -l "class UpdateCompanyCommand" -n .)
if [ -n "$update_file" ]; then
sed -n '1,20p' "$update_file"
else
echo "UpdateCompanyCommand.cs not found"
fiLength of output: 712
Standardize the shared request model and clarify its intent
The EditCompanyBodyRequest type is currently defined under the CreateCompany feature but is being reused by UpdateCompany. To avoid confusion and better reflect its dual role:
- Consolidate this DTO into a neutral name (for example,
CompanyBodyRequest) - Move it to a shared namespace/folder, e.g.
src/Web.Api/Features/Companies/Models/CompanyBodyRequest.cs - Update both
CreateCompanyCommand.csandUpdateCompanyCommand.csto import the new namespace:
- using Web.Api.Features.Companies.CreateCompany;
+ using Web.Api.Features.Companies.Models;This will make its purpose clear, reduce coupling between feature folders, and improve maintainability.
Committable suggestion skipped: line range outside the PR's diff.
| public void Update( | ||
| string name, | ||
| string description, | ||
| string logoUrl, | ||
| List<string> links) | ||
| { | ||
| name = name?.Trim(); | ||
| description = description?.Trim(); | ||
|
|
||
| if (string.IsNullOrWhiteSpace(name)) | ||
| { | ||
| throw new BadRequestException("Company name cannot be empty."); | ||
| } | ||
|
|
||
| if (string.IsNullOrWhiteSpace(description)) | ||
| { | ||
| throw new BadRequestException("Company description cannot be empty."); | ||
| } | ||
|
|
||
| Name = name; | ||
| NormalizedName = name.ToUpperInvariant(); | ||
| Description = description; | ||
| LogoUrl = logoUrl; | ||
| Links = links ?? new List<string>(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add check for deleted companies in Update method.
The Update method implementation looks good with proper validation and normalization of inputs. However, unlike other methods that modify the entity (AddReview, ApproveReview, MarkReviewAsOutdated), it doesn't check if the company has been deleted.
Consider adding a call to NotDeletedOrFail() at the beginning of the method:
public void Update(
string name,
string description,
string logoUrl,
List<string> links)
{
+ NotDeletedOrFail();
+
name = name?.Trim();
description = description?.Trim();
// rest of the method...
}
Summary by CodeRabbit
New Features
Improvements
Refactor
Other